[HVM] Fix an error when read from APIC registers like IRR, ISR and TMR.
authorkaf24@firebug.cl.cam.ac.uk <kaf24@firebug.cl.cam.ac.uk>
Wed, 13 Sep 2006 14:59:14 +0000 (15:59 +0100)
committerkaf24@firebug.cl.cam.ac.uk <kaf24@firebug.cl.cam.ac.uk>
Wed, 13 Sep 2006 14:59:14 +0000 (15:59 +0100)
From SDM3 spec, for APIC registers, all 32-bit registers should
be accessed using 128-bit aligned 32bit loads or stores.
And wider registers (64-bit or 256-bit) must be accessed using
multiple 32-bit loads or stores.

In old APIC virtualization code, we use IRR, ISR and TMR which are
256-bit registers as contiguous bit maps other than multiple 32-bit.

So guest always fetch error values.

Original patch was:
 * Signed-off-by: Xiaohui Xin <xiaohui.xin@intel.com>
 * Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com>
 * Signed-off-by: Eddie Dong <eddie.dong@intel.com>

Signed-off-by: Keir Fraser <keir@xensource.com>
xen/arch/x86/hvm/vlapic.c
xen/include/asm-x86/hvm/vlapic.h

index bedb8779424cffd77ffdfb1f000264c33c3eddca..a3ed24a491f81797f0d7557a5664a3242b42d54d 100644 (file)
@@ -66,12 +66,10 @@ int vlapic_find_highest_irr(struct vlapic *vlapic)
 {
     int result;
 
-     result = find_highest_bit((unsigned long *)(vlapic->regs + APIC_IRR),
-                               MAX_VECTOR);
+    result = vlapic_find_highest_vector(vlapic->regs + APIC_IRR);
+    ASSERT((result == -1) || (result >= 16));
 
-     ASSERT( result == -1 || result >= 16);
-
-     return result;
+    return result;
 }
 
 s_time_t get_apictime_scheduled(struct vcpu *v)
@@ -89,10 +87,8 @@ int vlapic_find_highest_isr(struct vlapic *vlapic)
 {
     int result;
 
-    result = find_highest_bit((unsigned long *)(vlapic->regs + APIC_ISR),
-                               MAX_VECTOR);
-
-    ASSERT( result == -1 || result >= 16);
+    result = vlapic_find_highest_vector(vlapic->regs + APIC_ISR);
+    ASSERT((result == -1) || (result >= 16));
 
     return result;
 }
@@ -221,7 +217,8 @@ static int vlapic_accept_irq(struct vcpu *v, int delivery_mode,
         if ( unlikely(vlapic == NULL || !vlapic_enabled(vlapic)) )
             break;
 
-        if ( test_and_set_bit(vector, vlapic->regs + APIC_IRR) && trig_mode)
+        if ( vlapic_test_and_set_vector(vector, vlapic->regs + APIC_IRR) &&
+             trig_mode)
         {
             HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                   "level trig mode repeatedly for vector %d\n", vector);
@@ -232,7 +229,7 @@ static int vlapic_accept_irq(struct vcpu *v, int delivery_mode,
         {
             HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
               "level trig mode for vector %d\n", vector);
-            set_bit(vector, vlapic->regs + APIC_TMR);
+            vlapic_set_vector(vector, vlapic->regs + APIC_TMR);
         }
         hvm_prod_vcpu(v);
 
@@ -358,10 +355,10 @@ void vlapic_EOI_set(struct vlapic *vlapic)
     if ( vector == -1 )
         return ;
 
-    clear_bit(vector, vlapic->regs + APIC_ISR);
+    vlapic_clear_vector(vector, vlapic->regs + APIC_ISR);
     vlapic_update_ppr(vlapic);
 
-    if ( test_and_clear_bit(vector, vlapic->regs + APIC_TMR) )
+    if ( vlapic_test_and_clear_vector(vector, vlapic->regs + APIC_TMR) )
         ioapic_update_EOI(vlapic->domain, vector);
 }
 
@@ -816,7 +813,7 @@ void vlapic_timer_fn(void *data)
 
     vlapic->timer_last_update = now;
 
-    if ( test_and_set_bit(timer_vector, vlapic->regs + APIC_IRR ))
+    if ( vlapic_test_and_set_vector(timer_vector, vlapic->regs + APIC_IRR) )
         vlapic->intr_pending_count[timer_vector]++;
 
     if ( vlapic_lvtt_period(vlapic) )
@@ -893,7 +890,7 @@ int cpu_get_apic_interrupt(struct vcpu *v, int *mode)
                 HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                             "Sending an illegal vector 0x%x.", highest_irr);
 
-                set_bit(err_vector, vlapic->regs + APIC_IRR);
+                vlapic_set_vector(err_vector, vlapic->regs + APIC_IRR);
                 highest_irr = err_vector;
             }
 
@@ -943,15 +940,15 @@ void vlapic_post_injection(struct vcpu *v, int vector, int deliver_mode)
     switch ( deliver_mode ) {
     case APIC_DM_FIXED:
     case APIC_DM_LOWEST:
-        set_bit(vector, vlapic->regs + APIC_ISR);
-        clear_bit(vector, vlapic->regs + APIC_IRR);
+        vlapic_set_vector(vector, vlapic->regs + APIC_ISR);
+        vlapic_clear_vector(vector, vlapic->regs + APIC_IRR);
         vlapic_update_ppr(vlapic);
 
         if ( vector == vlapic_lvt_vector(vlapic, APIC_LVTT) )
         {
             vlapic->intr_pending_count[vector]--;
             if ( vlapic->intr_pending_count[vector] > 0 )
-                test_and_set_bit(vector, vlapic->regs + APIC_IRR);
+                vlapic_test_and_set_vector(vector, vlapic->regs + APIC_IRR);
         }
         break;
 
index 7550bf3b05cb74d107ddccf4e24484e0bc814725..1da56bd0cae29f34f42a8f6b1bce4028f05f911d 100644 (file)
 #include <asm/msr.h>
 #include <public/hvm/ioreq.h>
 
-static __inline__ int find_highest_bit(unsigned long *data, int nr_bits)
+#define MAX_VECTOR      256
+
+#define VEC_POS(v) ((v)%32)
+#define REG_POS(v) (((v)/32)* 0x10)
+#define vlapic_test_and_set_vector(vec, bitmap)                 \
+    test_and_set_bit(VEC_POS(vec), (bitmap) + REG_POS(vec))
+#define vlapic_test_and_clear_vector(vec, bitmap)               \
+    test_and_clear_bit(VEC_POS(vec), (bitmap) + REG_POS(vec))
+#define vlapic_set_vector(vec, bitmap)                          \
+    set_bit(VEC_POS(vec), (bitmap) + REG_POS(vec))
+#define vlapic_clear_vector(vec, bitmap)                        \
+    clear_bit(VEC_POS(vec), (bitmap) + REG_POS(vec))
+
+static inline int vlapic_find_highest_vector(u32 *bitmap)
 {
-    int length = BITS_TO_LONGS(nr_bits);
-    while ( length && !data[--length] )
+    int word_offset = MAX_VECTOR / 32;
+
+    /* Work backwards through the bitmap (first 32-bit word in every four). */
+    while ( (word_offset != 0) && (bitmap[(--word_offset)*4] == 0) )
         continue;
-    return (fls(data[length]) - 1) + (length * BITS_PER_LONG);
+
+    return (fls(bitmap[word_offset*4]) - 1) + (word_offset * 32);
 }
 
 #define VLAPIC(v)                       (v->arch.hvm_vcpu.vlapic)
@@ -83,8 +99,6 @@ typedef struct direct_intr_info {
     int source[6];
 } direct_intr_info_t;
 
-#define MAX_VECTOR      256
-
 struct vlapic {
     uint32_t           status;
     uint32_t           vcpu_id;
@@ -108,9 +122,9 @@ static inline int vlapic_set_irq(struct vlapic *vlapic,
 {
     int ret;
 
-    ret = test_and_set_bit(vec, vlapic->regs + APIC_IRR);
+    ret = vlapic_test_and_set_vector(vec, vlapic->regs + APIC_IRR);
     if ( trig )
-        set_bit(vec, vlapic->regs + APIC_TMR);
+        vlapic_set_vector(vec, vlapic->regs + APIC_TMR);
 
     /* We may need to wake up target vcpu, besides set pending bit here */
     return ret;